Optimize decompression (on ARM) up to 2x - #64
Conversation
|
@BurntSushi I am wondering, what is your current capacity in reviewing/taking a change like this (when cleaned up)? In arrow-rs we depend on |
| ptr::copy_nonoverlapping(srcp, dst, 8); | ||
| ptr::copy_nonoverlapping(srcp.add(8), dst.add(8), 8); |
There was a problem hiding this comment.
During my testing, I noticed that the implementation of this part on ARM differs from that on x86. Is there a better implementation?
There was a problem hiding this comment.
Not sure, this was something that resulted in better codegen (according to claude), but I didn't run the benchmarks on x86.
I think for some parts it seemed a bit harder to generate great code for both with one implementation.
I think it probably would make sense to take some ideas from this PR and port them individually in order to improve the performance.
| let src_len = self.src.len(); | ||
| let dst_len = self.dst.len(); | ||
|
|
||
| if src_len < 17 || dst_len < 88 { |
There was a problem hiding this comment.
These two numbers seem a bit odd. Could you please provide further clarification? My understanding is as follows:
17: src ≥ 17 bytes: 1-byte tag + up to 16 bytes of literal data; no need to check the remaining space in src within the loop
88: dst ≥ 88 bytes: 64-byte maximum copy + 24 bytes of overwrite margin; no need to check the remaining space in dst within the loop
There was a problem hiding this comment.
Yeah - it is a bit magic created by vibe code ;) but it is headroom for the rest of the loop to avoid checking space.
| fn extract_offset_mask(tag_type: usize) -> u32 { | ||
| #[cfg(target_arch = "aarch64")] | ||
| { | ||
| const MASKS_PACKED: u64 = 0x0000FFFF00FF0000u64; | ||
| ((MASKS_PACKED >> (tag_type * 16)) & 0xFFFF) as u32 | ||
| } | ||
| #[cfg(not(target_arch = "aarch64"))] | ||
| { | ||
| const MASKS: [u32; 4] = [0, 0xFF, 0xFFFF, 0]; | ||
| MASKS[tag_type] | ||
| } | ||
| } |
There was a problem hiding this comment.
Would using bitwise operations throughout affect X86 performance?
There was a problem hiding this comment.
I think it did generate worse code (but didn't run benchmarks)
Some claude-assisted optimizations.
On M1 CPU: